packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (3)

9-9: Import naming could be clearer after class rename.

The import statement imports the class as EndpointManager from endpointManager2.ts, but the class was renamed to EndpointManager2 in that file. While this works, it might be clearer to import it with its actual name for consistency.

Consider importing with the actual class name:

-import EndpointManager from '@polkadot/extension-polkagate/src/class/endpointManager2';
+import EndpointManager2 from '@polkadot/extension-polkagate/src/class/endpointManager2';

And update line 16:

-const endpointManager = new EndpointManager();
+const endpointManager = new EndpointManager2();

---

180-207: Consider handling non-WSS endpoints.

The function only handles auto-mode and WSS endpoints (lines 198-206). If an endpoint doesn't start with 'wss' and isn't auto-mode, it will be marked as requested but no connection will be initiated. This could leave pending connections unresolved.

Consider adding a fallback or error handling for unsupported endpoints:

   if (endpoint.endpoint.startsWith('wss')) {
     connectToEndpoint(endpoint.endpoint).catch(console.error);
+  } else {
+    console.warn('Unsupported endpoint type:', endpoint.endpoint);
+    resolvePendingConnections(genesisHash, undefined);
   }
 }, [connectToEndpoint, handleAutoMode]);

---

64-269: Consider adding cleanup for disconnected APIs.

The component maintains a map of APIs but doesn't clean up disconnected APIs or disconnect them on unmount. This could lead to memory leaks over time, especially if users frequently switch between chains.

Consider adding a cleanup effect:

useEffect(() => {
  return () => {
    // Cleanup on unmount
    Object.values(apis).forEach((apiList) => {
      apiList?.forEach(({ api }) => {
        if (api?.isConnected) {
          api.disconnect().catch(console.error);
        }
      });
    });
  };
}, []); // Empty deps - only run on unmount

Additionally, consider periodically removing disconnected APIs from the map to prevent unbounded growth.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: AMIRKHANEF
PR: PolkaGate/extension#1467
File: packages/extension-polkagate/src/util/types.ts:0-0
Timestamp: 2024-08-31T05:19:02.468Z
Learning: The `SavedEndpoint` interface in the `packages/extension-polkagate/src/util/types.ts` file has a property named `isOnManual`, which indicates whether the endpoint is in manual mode.

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (4)

86-93: Don’t hardcode isAuto; persist manual vs auto correctly.

updateEndpoint always sets isAuto: true, which is wrong for manual selections and prevents accurate mode tracking.

-const updateEndpoint = useCallback((chainKey: string, selectedEndpoint: string, cbFunction?: () => void) => {
+const updateEndpoint = useCallback((
+  genesisHash: string,
+  selectedEndpoint: string,
+  isAuto: boolean,
+  cbFunction?: () => void
+) => {
   try {
     const newEndpoint = {
       checkForNewOne: false,
       endpoint: selectedEndpoint,
-      isAuto: true,
+      isAuto,
       timestamp: Date.now()
     };
-
-    endpointManager.set(chainKey, newEndpoint);
+    endpointManager.set(genesisHash, newEndpoint);
     cbFunction?.();
   } catch (error) {
     console.error(error);
   }
 }, []);

And pass isAuto=true only from auto flow (see Line 158), false from manual flow.

---

161-178: On error, resolve the correct pending and free resources.

connectToEndpoint guesses a genesisHash and may resolve the wrong pending; also leaks wsProvider on failure.

-const connectToEndpoint = useCallback(async (endpointToConnect: string) => {
+const connectToEndpoint = useCallback(async (genesisHash: string, endpointToConnect: string) => {
   try {
     const wsProvider = new WsProvider(endpointToConnect);
     const newApi = await ApiPromise.create({ provider: wsProvider });
 
     handleNewApi(newApi, endpointToConnect);
   } catch (error) {
     console.error('Connection error:', error);
-    // Resolve pending with undefined on error
-    const genesisHash = Object.keys(pendingConnections.current).find((key) =>
-      pendingConnections.current[key].length > 0
-    );
-
-    if (genesisHash) {
-      resolvePendingConnections(genesisHash, undefined);
-    }
+    // Resolve only the intended pending
+    resolvePendingConnections(genesisHash, undefined);
   }
 }, [handleNewApi, resolvePendingConnections]);

Also update the call site (Line 205) to pass genesisHash and disconnect the provider on failure if instantiated before create().

- connectToEndpoint(endpoint.endpoint).catch(console.error);
+ connectToEndpoint(genesisHash, endpoint.endpoint).catch(console.error);

---

236-241: Pending keyed only by genesis can resolve a different endpoint.

Known issue (TODO). Track pending by both genesis and endpoint (or store endpoint on each entry and validate), so callers don’t receive an API for a different endpoint.

-const pendingConnections = useRef<
-  Record<string, {
-    promise: Promise<ApiPromise | undefined>;
-    resolve(api: ApiPromise | undefined): void;
-  }[]>
->({});
+type PendingEntry = { promise: Promise<ApiPromise | undefined>; resolve(api: ApiPromise | undefined): void };
+const pendingConnections = useRef<Record<string, Record<string, PendingEntry>>>({});

Adjust creation/lookup:

-if (pendingConnections.current[genesisHash]?.length > 0) {
-  return pendingConnections.current[genesisHash][0].promise;
-}
+const pendByEp = pendingConnections.current[genesisHash];
+if (pendByEp?.[endpoint.endpoint!]) {
+  return pendByEp[endpoint.endpoint!].promise;
+}
@@
-if (!pendingConnections.current[genesisHash]) {
-  pendingConnections.current[genesisHash] = [];
-}
-
-pendingConnections.current[genesisHash].push({
-  promise,
-  resolve: resolvePromise
-});
+pendingConnections.current[genesisHash] ||= {};
+pendingConnections.current[genesisHash][endpoint.endpoint!] = { promise, resolve: resolvePromise };

And in resolvePendingConnections, remove by endpoint:

-const resolvePendingConnections = useCallback((genesisHash: string, api: ApiPromise | undefined) => {
-  const pending = pendingConnections.current[genesisHash];
-  if (pending) {
-    pending.forEach(({ resolve }) => resolve(api));
-    delete pendingConnections.current[genesisHash];
-  }
-}, []);
+const resolvePendingConnections = useCallback((genesisHash: string, api: ApiPromise | undefined, endpoint?: string) => {
+  const pendByEp = pendingConnections.current[genesisHash];
+  if (!pendByEp) return;
+  if (endpoint && pendByEp[endpoint]) {
+    pendByEp[endpoint].resolve(api);
+    delete pendByEp[endpoint];
+  } else {
+    // fallback: resolve all
+    Object.values(pendByEp).forEach(({ resolve }) => resolve(api));
+    delete pendingConnections.current[genesisHash];
+  }
+}, []);

Call with endpoint where available (e.g., in handleNewApi and error paths).

---

140-160: updateEndpoint call should pass isAuto=true explicitly.

After changing updateEndpoint’s signature, update this call.

- updateEndpoint(genesisHash, selectedEndpoint, () => handleNewApi(api, selectedEndpoint, true));
+ updateEndpoint(genesisHash, selectedEndpoint, true, () => handleNewApi(api, selectedEndpoint, true));

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (2)

102-110: resolvePendingConnections should accept endpoint to avoid cross-resolution.

Update callers to pass the endpoint so only that pending resolves.

-const resolvePendingConnections = useCallback((genesisHash: string, api: ApiPromise | undefined) => {
+const resolvePendingConnections = useCallback((genesisHash: string, api: ApiPromise | undefined, endpoint?: string) => {
   const pending = pendingConnections.current[genesisHash];
-  if (pending) {
-    pending.forEach(({ resolve }) => resolve(api));
-    delete pendingConnections.current[genesisHash];
-  }
+  // ...see diff in the main pending refactor comment...
 }, []);

Update calls at Lines 137, 153, 175 accordingly.

---

70-77: Prefer Set for requestedQueue.

Use Set per genesis for O(1) checks and easy deletion.

-const requestedQueue = useRef<Record<string, string[]>>({});
+const requestedQueue = useRef<Record<string, Set<string>>>({});

Then adapt includes/push/filter usages accordingly.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (6)

8-16: Import alias mismatches exported class name.

The file endpointManager2.ts exports EndpointManager2 as its default export, but line 9 aliases it as EndpointManager. This creates confusion about which version is in use and masks the actual class name.

Align the import with the exported name:

-import EndpointManager from '@polkadot/extension-polkagate/src/class/endpointManager2';
+import EndpointManager2 from '@polkadot/extension-polkagate/src/class/endpointManager2';

Then update line 16 and any other usages:

-const endpointManager = new EndpointManager();
+const endpointManager = new EndpointManager2();

---

86-100: Hardcoded isAuto: true incorrect for manual endpoint selections.

The updateEndpoint function always sets isAuto: true (line 91), even when called for manually-selected endpoints. This misrepresents the selection mode and could cause incorrect behavior in endpoint management.

Add an isAuto parameter:

-const updateEndpoint = useCallback((chainKey: string, selectedEndpoint: string, cbFunction?: () => void) => {
+const updateEndpoint = useCallback((chainKey: string, selectedEndpoint: string, isAuto: boolean, cbFunction?: () => void) => {
   try {
     const newEndpoint = {
       checkForNewOne: false,
       endpoint: selectedEndpoint,
-      isAuto: true,
+      isAuto,
       timestamp: Date.now()
     };

Then update the call site at line 158:

-updateEndpoint(genesisHash, selectedEndpoint, () => handleNewApi(api, selectedEndpoint, true));
+updateEndpoint(genesisHash, selectedEndpoint, true, () => handleNewApi(api, selectedEndpoint, true));

---

112-138: Clear requestedQueue flags after successful connection.

The requestedQueue marks endpoints as requested but entries are never removed after a successful connection. This prevents future retry attempts for the same endpoint and causes a resource leak.

Clear the request flag after handling:

 const handleNewApi = useCallback((api: ApiPromise, endpoint: string, onAutoMode?: boolean) => {
   const genesisHash = String(api.genesisHash.toHex());

   setApis((prevApis) => {
     // ... existing logic ...
   });

+  // Clear request mark for this endpoint
+  const rq = requestedQueue.current[genesisHash];
+  if (rq) {
+    requestedQueue.current[genesisHash] = rq.filter((e) => e !== endpoint);
+  }
+
   // Resolve all waiting promises
   resolvePendingConnections(genesisHash, api);
 }, [resolvePendingConnections]);

---

174-201: Manual endpoint selections are not persisted.

The manual selection path (line 199) calls connectToEndpoint but never persists the chosen endpoint via updateEndpoint, unlike the auto-mode path. This means manual selections won't be remembered across sessions or component remounts.

Persist manual selections and handle failures:

   if (endpoint.endpoint.startsWith('wss')) {
-    connectToEndpoint(genesisHash, endpoint.endpoint).catch(console.error);
+    connectToEndpoint(genesisHash, endpoint.endpoint)
+      .then(() => {
+        // Persist manual selection (isAuto: false)
+        // Note: updateEndpoint signature needs the isAuto parameter added first
+      })
+      .catch((e) => {
+        console.error(e);
+        // Clear request mark on failure
+        const rq = requestedQueue.current[genesisHash];
+        if (rq) {
+          requestedQueue.current[genesisHash] = rq.filter((v) => v !== endpoint.endpoint);
+        }
+      });
   }

Note: This requires fixing the updateEndpoint signature first to accept an isAuto parameter.

---

208-214: Avoid writing default AUTO_MODE to storage inside getApi.

Writing the default AUTO_MODE_DEFAULT_ENDPOINT to storage immediately (lines 211-213) when no endpoint is found can overwrite user settings during startup before storage load completes in EndpointManager2. This mutation should not happen in a getter function.

Treat the default as ephemeral:

   let endpoint = endpointManager.get(genesisHash);

   if (!endpoint) {
-    endpoint = { ...AUTO_MODE_DEFAULT_ENDPOINT, timestamp: Date.now() };
-
-    endpointManager.set(genesisHash, endpoint);
+    // Use as in-memory default; do not persist yet
+    endpoint = { ...AUTO_MODE_DEFAULT_ENDPOINT, timestamp: Date.now() };
   }

Let the endpoint be persisted later by handleAutoMode via updateEndpoint when auto-mode actually selects an endpoint, or by explicit user action for manual selections.

---

230-235: Pending connection may resolve to wrong endpoint.

The pendingConnections structure is keyed only by genesisHash, not endpoint. If getApi is called multiple times with different endpoints for the same genesis hash while a connection is pending, all callers share the first pending promise, which may resolve to an API for a different endpoint than requested.

The TODO comment at line 230 correctly identifies this issue. Consider one of these solutions:

Solution 1: Track endpoint in pending connections:

 const pendingConnections = useRef<
-  Record<string, {
+  Record<string, Record<string, {
     promise: Promise<ApiPromise | undefined>;
     resolve(api: ApiPromise | undefined): void;
-  }[]>
+  }>>
 >({});

Then key by both genesisHash and endpoint, checking that the pending endpoint matches before returning the promise.

Solution 2: Include endpoint validation in the existing array structure by storing endpoint alongside each pending entry and filtering for matching endpoint before returning the promise.

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (1)

237-252: Consider standard Promise pattern for clarity.

The pattern of initializing resolvePromise with a no-op (line 240) then overwriting it in the executor is safe but unconventional. A more standard pattern would eliminate the intermediate assignment.

-  let resolvePromise: (api: ApiPromise | undefined) => void = () => undefined;
-  const promise = new Promise<ApiPromise | undefined>((resolve) => {
-    resolvePromise = resolve;
-  });
+  const entry = (() => {
+    let resolvePromise: (api: ApiPromise | undefined) => void;
+    const promise = new Promise<ApiPromise | undefined>((resolve) => {
+      resolvePromise = resolve;
+    });
+    return { promise, resolve: resolvePromise! };
+  })();

   if (!pendingConnections.current[genesisHash]) {
     pendingConnections.current[genesisHash] = [];
   }

-  pendingConnections.current[genesisHash].push({
-    promise,
-    resolve: resolvePromise
-  });
+  pendingConnections.current[genesisHash].push(entry);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (6)

109-135: Clear requestedQueue on success to allow future retries.

   setApis((prevApis) => {
@@
   });
+  // unmark request for this endpoint
+  const rq = requestedQueue.current[genesisHash];
+  if (rq) {
+    requestedQueue.current[genesisHash] = rq.filter((e) => e !== endpoint);
+  }
   resolvePendingConnections(genesisHash, api);

---

158-169: On connection error, clear requestedQueue for this endpoint.

   } catch (error) {
     console.error('Connection error:', error);
-    // Resolve pending with undefined on error
+    // Clear request mark on failure
+    const rq = requestedQueue.current[genesisHash];
+    if (rq) {
+      requestedQueue.current[genesisHash] = rq.filter((e) => e !== endpointToConnect);
+    }
+    // Resolve pending with undefined on error
     resolvePendingConnections(genesisHash, undefined);
   }

---

171-198: Persist manual selections and cleanup request flags.

-  if (isAutoMode(endpoint.endpoint)) {
+  if (isAutoMode(endpoint.endpoint)) {
     handleAutoMode(genesisHash, endpoints).catch(console.error);
     return;
   }
 
   if (endpoint.endpoint.startsWith('wss')) {
-    connectToEndpoint(genesisHash, endpoint.endpoint).catch(console.error);
+    // Persist manual selection immediately
+    updateEndpoint(genesisHash, endpoint.endpoint, false);
+    connectToEndpoint(genesisHash, endpoint.endpoint)
+      .catch((e) => {
+        console.error(e);
+        // clear request mark on failure
+        const rq = requestedQueue.current[genesisHash];
+        if (rq) {
+          requestedQueue.current[genesisHash] = rq.filter((v) => v !== endpoint.endpoint);
+        }
+      });
   }
-}, [connectToEndpoint, handleAutoMode]);
+}, [connectToEndpoint, handleAutoMode, updateEndpoint]);

---

83-91: Don’t hardcode isAuto: true in updateEndpoint.

-const updateEndpoint = useCallback((chainKey: string, selectedEndpoint: string, cbFunction?: () => void) => {
+const updateEndpoint = useCallback((chainKey: string, selectedEndpoint: string, isAuto: boolean, cbFunction?: () => void) => {
   try {
     const newEndpoint = {
       checkForNewOne: false,
       endpoint: selectedEndpoint,
-      isAuto: true,
+      isAuto,
       timestamp: Date.now()
     };
     endpointManager.set(chainKey, newEndpoint);
     cbFunction?.();
   } catch (error) {
     console.error(error);
   }
-}, []);
+}, []);

Update callers accordingly (auto: true; manual: false).

---

200-211: Avoid writing default AUTO endpoint inside getApi; can clobber storage during init.

-  if (!endpoint) {
-    endpoint = { ...AUTO_MODE_DEFAULT_ENDPOINT, timestamp: Date.now() };
-    endpointManager.set(genesisHash, endpoint);
-  }
+  if (!endpoint) {
+    // Use ephemeral default; do not persist here
+    endpoint = { ...AUTO_MODE_DEFAULT_ENDPOINT, timestamp: Date.now() };
+  }

Persist later via updateEndpoint after a successful auto/manual selection.

---

227-233: Pending promise keyed only by genesisHash can resolve wrong endpoint.

Track endpoint in pending entries and only reuse/resolve matching ones.

-  if (pendingConnections.current[genesisHash]?.length > 0) {
-    return pendingConnections.current[genesisHash][0].promise;
-  }
+  const pendings = pendingConnections.current[genesisHash] || [];
+  const epKey = endpoint.endpoint; // 'AutoMode' or concrete wss
+  const existing = pendings.find((p) => p.endpoint === epKey);
+  if (existing) return existing.promise;

And update the type and push site:

-const pendingConnections = useRef<
-  Record<string, {
+const pendingConnections = useRef<
+  Record<string, {
     promise: Promise<ApiPromise | undefined>;
     resolve(api: ApiPromise | undefined): void;
-  }[]>
+    endpoint: string; // key (AutoMode or wss)
+  }[]>
 >({});

At push:

- pendingConnections.current[genesisHash].push({ promise, resolve: resolvePromise });
+ pendingConnections.current[genesisHash].push({ promise, resolve: resolvePromise, endpoint: epKey });

Update resolve to accept optional endpoint to target specific waiters.

packages/extension-polkagate/src/util/workers/getValidatorsInfo.js (2)

35-39: Avoid JSON deep-clone before postMessage; structured clone is automatic.

postMessage already structured‑clones. JSON clone is slower and can lose types.

-  return JSON.parse(JSON.stringify({
-    current: nextElectedInfo,
-    eraIndex: Number(currentEra),
-    waiting: waiting.info
-  }));
+  return {
+    current: nextElectedInfo,
+    eraIndex: Number(currentEra?.toString?.() ?? currentEra),
+    waiting: waiting.info
+  };

---

40-44: Use console.error for failures.

Use error log level to aid monitoring.

-  } catch (error) {
-    console.log('something went wrong while getting validators info, err:', error);
+  } catch (error) {
+    console.error('something went wrong while getting validators info, err:', error);

packages/extension-polkagate/src/fullscreen/settings/partials/useEndpointsSetting.ts (1)

144-157: Reset isFetching on failure; use finally.

Currently, failures keep the endpoint marked “fetching” forever; no retries.

-  endpoint && CalculateNodeDelay(endpoint)
-    .then((response) => {
+  endpoint && CalculateNodeDelay(endpoint)
+    .then((response) => {
       if (!response) {
         return;
       }
-
       isFetching.current[endpoint] = false;
       dispatch({ payload: { delay: response.delay, endpoint: response.endpoint }, type: 'UPDATE_DELAY' });
-
       response.api?.disconnect().catch(console.error);
-    })
-    .catch(console.error);
+    })
+    .catch(console.error)
+    .finally(() => { isFetching.current[endpoint] = false; });

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (1)

219-225: Prefer returning api matching the saved endpoint (or auto) instead of any connected.

Today it returns the first connected api, which may not match the current selection.

-const availableApi = apiList?.find(({ api }) => api?.isConnected);
+const saved = endpointManager.get(genesisHash);
+const availableApi = saved?.endpoint && !isAutoMode(saved.endpoint)
+  ? apiList?.find(({ endpoint, api }) => endpoint === saved.endpoint && api?.isConnected)
+  : apiList?.find(({ api }) => api?.isConnected);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: AMIRKHANEF
PR: PolkaGate/extension#1467
File: packages/extension-polkagate/src/util/types.ts:0-0
Timestamp: 2024-08-31T05:19:02.468Z
Learning: The `SavedEndpoint` interface in the `packages/extension-polkagate/src/util/types.ts` file has a property named `isOnManual`, which indicates whether the endpoint is in manual mode.

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (8)

83-97: Hardcoded isAuto: true causes incorrect mode tracking.

As previously noted, updateEndpoint always sets isAuto: true regardless of whether the selection was automatic or manual. This breaks endpoint mode tracking.

---

99-107: Resolves all pending connections regardless of endpoint.

As previously noted, this function resolves all pending promises for a genesisHash without checking if they're waiting for the same endpoint. This causes the issue mentioned in the TODO at line 227.

---

109-135: Missing cleanup of requestedQueue on successful connection.

As previously noted, successful connections don't clear the requestedQueue flag, preventing future reconnection attempts if the connection drops.

---

137-156: Short-circuit check never matches because saved endpoints are wss URLs.

As previously noted, line 139 checks if any saved endpoint string equals AUTO_MODE.value, but saved APIs store wss URLs (line 155), not the "AutoMode" string. The check should query endpointManager.get(genesisHash)?.isAuto or store an isAuto flag on each saved API entry.

---

158-169: Error handler doesn't clear requestedQueue flag.

As previously noted, when connection fails, the requestedQueue flag remains set, blocking future retry attempts for this endpoint.

---

171-198: Manual endpoint selection not persisted; no error cleanup.

As previously noted, the manual selection path (lines 195-197) doesn't persist the endpoint choice and doesn't clear requestedQueue flags on connection failure.

---

207-211: Avoid persisting AUTO_MODE default inside getApi.

As previously noted, writing the default endpoint to storage during getApi can overwrite user settings during startup. Treat AUTO_MODE_DEFAULT_ENDPOINT as an ephemeral default and persist only after successful auto-selection or explicit user action.

---

227-232: Pending connections not keyed by endpoint.

As previously noted and acknowledged by the TODO comment, pending promises are keyed only by genesisHash. If multiple callers request different endpoints for the same genesis, they all receive the same promise, which may resolve to an API for the wrong endpoint.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (7)

83-97: Still hardcoded: isAuto: true prevents correct tracking of manual selections.

The function always sets isAuto: true on line 88, which was flagged in a previous review. Although the current call site (line 163) is in an auto-mode context, the function signature suggests general-purpose use. If future code calls updateEndpoint for a manual selection, the mode will be incorrectly recorded.

Apply the previously suggested fix:

-const updateEndpoint = useCallback((genesisHash: string, selectedEndpoint: string, cbFunction?: () => void) => {
+const updateEndpoint = useCallback((genesisHash: string, selectedEndpoint: string, isAuto: boolean, cbFunction?: () => void) => {
   try {
     const newEndpoint = {
       checkForNewOne: false,
       endpoint: selectedEndpoint,
-      isAuto: true,
+      isAuto,
       timestamp: Date.now()
     };

Update the call site at line 163:

-updateEndpoint(genesisHash, selectedEndpoint, () => handleNewApi(api, selectedEndpoint, true));
+updateEndpoint(genesisHash, selectedEndpoint, true, () => handleNewApi(api, selectedEndpoint, true));

---

99-116: Resolves all pending promises regardless of endpoint.

Line 104 resolves every pending promise for the genesisHash, even though they may be waiting for different endpoints. If caller A awaits getApi(genesis, endpointX) and caller B awaits getApi(genesis, endpointY) concurrently, resolving one will incorrectly resolve both with the same API instance.

The request-flag clearing (lines 108-115) is correct, but the promise resolution needs similar selectivity. Consider tracking the endpoint in pendingConnections and resolving only matching entries, as suggested in the previous review.

---

145-164: autoModeExists check never matches; duplicate auto-mode connections possible.

Line 147 checks isAutoMode(endpoint) on stored API entries, but handleNewApi saves APIs with their actual wss endpoint string (line 130-133), not the "AutoMode" literal. This check always returns false, so repeated handleAutoMode calls for the same genesis will spawn redundant connections.

As previously suggested, check the EndpointManager state or store an isAuto flag alongside each saved API entry:

-  const autoModeExists = apisForGenesis?.some(({ endpoint }) => isAutoMode(endpoint));
+  const storedEndpoint = endpointManager.get(genesisHash);
+  const autoModeExists = storedEndpoint?.isAuto === true &&
+    (apisForGenesis?.some(({ api }) => api?.isConnected) ?? false);

---

201-203: Manual endpoint selections are not persisted.

When a user manually selects a wss endpoint (lines 201-203), the connection is initiated but the selection is never saved via updateEndpoint. On reload or future calls, the user's choice will be lost. This was flagged in a previous review and remains unaddressed.

Persist the manual selection:

   if (endpointValue.startsWith('wss')) {
-    connectToEndpoint(genesisHash, endpointValue).catch(console.error);
+    updateEndpoint(genesisHash, endpointValue, false);
+    connectToEndpoint(genesisHash, endpointValue).catch(console.error);
   }

Add updateEndpoint to the dependency array:

-}, [connectToEndpoint, handleAutoMode]);
+}, [connectToEndpoint, handleAutoMode, updateEndpoint]);

Note: This also requires fixing the updateEndpoint signature to accept the isAuto parameter as noted earlier.

---

211-217: Still writing default endpoint to storage; marked as addressed but persists.

Lines 213-216 write AUTO_MODE_DEFAULT_ENDPOINT to storage when no endpoint is found. A previous review comment (marked as addressed) flagged this mutation, noting it can overwrite user settings during startup before storage loads. The issue remains in the current code.

Treat the default as ephemeral:

   let endpoint = endpointManager.get(genesisHash);

   if (!endpoint) {
-    endpoint = { ...AUTO_MODE_DEFAULT_ENDPOINT, timestamp: Date.now() };
-
-    endpointManager.set(genesisHash, endpoint);
+    endpoint = AUTO_MODE_DEFAULT_ENDPOINT; // use in-memory; do not persist yet
   }

Persistence will happen later in handleAutoMode via updateEndpoint(..., true) or on explicit user action.

---

219-223: Dead code: endpoint is always defined after line 213.

Lines 213-216 ensure endpoint is set to AUTO_MODE_DEFAULT_ENDPOINT when undefined. The subsequent check on line 219 is unreachable.

Remove the dead code:

-  if (!endpoint) {
-    console.warn('No endpoint found for', genesisHash);
-
-    return Promise.resolve(undefined);
-  }
-

---

233-238: Pending connection may resolve to API for a different endpoint.

The TODO comment on line 233 correctly identifies the issue: pendingConnections is keyed only by genesisHash, so concurrent calls with different endpoints for the same genesis will share the same pending promise, potentially returning an API connected to the wrong endpoint.

This issue was flagged in a previous review. Consider tracking the endpoint within pendingConnections:

 const pendingConnections = useRef<
-  Record<string, {
+  Record<string, Record<string, {
     promise: Promise<ApiPromise | undefined>;
     resolve(api: ApiPromise | undefined): void;
-  }[]>
+  }>>
 >({});

Then key by both genesisHash and endpoint, or validate that the pending entry's endpoint matches the requested one before returning its promise.

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (2)

182-182: Redundant check: endpointManager is always defined.

endpointManager is initialized as a module-level constant (line 16), so the check !endpointManager on line 182 is always false. Remove this condition:

-  if (!endpointValue || !endpointManager) {
+  if (!endpointValue) {
     return;
   }

---

243-246: Unnecessary resolvePromise initialization pattern.

The comment on line 242 explains initializing resolvePromise with a noop "so it's always defined," but this is unnecessary. The Promise executor (line 244) runs synchronously, so resolvePromise is assigned on line 245 before the variable is used on line 254.

Simplify:

-  // initialize resolvePromise with a noop so it's always defined for the push below,
-  // then overwrite it synchronously inside the Promise executor.
-  let resolvePromise: (api: ApiPromise | undefined) => void = () => undefined;
+  let resolvePromise: (api: ApiPromise | undefined) => void;
   const promise = new Promise<ApiPromise | undefined>((resolve) => {
     resolvePromise = resolve;
   });

TypeScript's definite assignment analysis understands the Promise executor runs immediately.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/extension-ui/src/Popup/contexts/ApiProvider.tsx (4)

8-9: Import alias mismatch flagged in previous review.

Previous review identified that endpointManager2.ts exports EndpointManager2 but you're importing it as EndpointManager. This remains unaddressed.

---

84-98: Hardcoded isAuto: true flagged in previous review.

Previous review noted that line 89 hardcodes isAuto: true, which is incorrect for manual endpoint selections. This issue remains unaddressed—the function signature should accept an isAuto parameter.

---

224-235: Storage write of default endpoint flagged in previous review.

Line 234 writes the AUTO_MODE_DEFAULT_ENDPOINT to storage when no endpoint exists. Previous review commented that this "can overwrite user settings during startup before storage load completes" and suggested treating the default as ephemeral. This was marked as "Addressed in commits a708d04 to adcf3d0" but the storage write is still present.

---

190-222: Handle unexpected endpoint formats.

If endpointValue doesn't start with "wss" or "light" and isn't auto-mode (e.g., "http://", "ws://"), line 204 marks it as requested but no connection is initiated. This blocks future retries for that endpoint.

Add a catch-all after line 221:

   if (endpointValue.startsWith('light')) {
     connectToLightClient(genesisHash, endpointValue).catch(console.error);
   }
+
+  // Unsupported endpoint format
+  if (!isAutoMode(endpointValue) && !endpointValue.startsWith('wss') && !endpointValue.startsWith('light')) {
+    console.warn('Unsupported endpoint format:', endpointValue);
+    // Clear request mark to allow retry
+    requestedQueue.current[genesisHash] = requestedQueue.current[genesisHash].filter((e) => e !== endpointValue);
+  }
 }, [connectToEndpoint, connectToLightClient, handleAutoMode]);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro